Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Updates in readme documentation #744

Closed
wants to merge 7 commits into from

Conversation

shimilgithub
Copy link

AthulyaMS
AthulyaMS previously approved these changes Nov 2, 2023
@Joel-Joseph-George Joel-Joseph-George self-requested a review November 2, 2023 14:56
Copy link
Collaborator

@kavitharaju kavitharaju left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Regarding the content I am not cent percent sure. may be run it though Joel Mathew also once after merging

README.md Outdated
Comment on lines 3 to 15
The server application that provides REST APIs to interact with the underlying Databases (SQL and Graph) and modules in Vachan-Engine.
<h1 align="center">
<br>
<a href="http://www.amitmerchant.com/electron-markdownify"><img src="https://api.vachanengine.org/static/images/Logo.svg" alt="vachanapi" width="200"></a>
<br>
Vachan API
<br>
</h1>
<h4 align="center">Unified API for <a href="https://api.vachanengine.org/" target="_blank">Vachan-Engine applications</a></h4>
<p align="center">
•<a href="#implementation-details"> Implementation Details</a> •
<a href="#start-app-with-docker">Start App</a> •
<a href="#set-up-locally-for-development-and-testingwithout-docker">Setup</a> •
<a href="#license">License</a>
</p>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Markdown format is often used for its simplicity. I thinks its better to stick to it and avoid embedded HTML as much as possible.
In these we can use markdown features to get decent level of formatting

# big heading
## small heading
### smaller heading
[link-text](link-target)
- list item1
- list item2
   - sub item1
   - sub item2

big heading

small heading

smaller heading

link-text

  • list item1
  • list item2
    • sub item1
    • sub item2

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kavitharaju Markdown doesn't directly provide the option to align text and resize images. HTML is used for this purpose.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How critical is aligning text in a readme?
About image resizing, yes you could use it places where markdown support is not there. But keep it limited to that

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another thing is I am reviewing this PR with little context. The parent issue says

Enhance the Readme file of Vachan API by providing comprehensive and clear documentation.

Which in my opinion doesn't say anything! What are we making more comprehensive and clear here? The issue itself should be more specific and to the point than a too generalized statement like this.

And honestly I doubt the changes in the PR is making anything more comprehensive and clear.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Something I liked here is the idea of adding a gif. Makes the document more interesting. But is the API docs the best thing we can show in it? May be... I dont know..

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How critical is aligning text in a readme? About image resizing, yes you could use it places where markdown support is not there. But keep it limited to that

Text alignment is required only at the single line description("Unified API for VE applications") and the direct links mentioned below that line.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does it really have to be center? Why?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

to keep it properly down to the logo(which is kept in middle)

Copy link
Author

@shimilgithub shimilgithub Nov 3, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another thing is I am reviewing this PR with little context. The parent issue says

Enhance the Readme file of Vachan API by providing comprehensive and clear documentation.

Which in my opinion doesn't say anything! What are we making more comprehensive and clear here? The issue itself should be more specific and to the point than a too generalized statement like this.

And honestly I doubt the changes in the PR is making anything more comprehensive and clear.

will modify issue description

Comment on lines 1 to 7
<h1 align="center">
<br>
<a href="http://www.amitmerchant.com/electron-markdownify"><img src="https://api.vachanengine.org/static/images/Logo.svg" alt="vachanapi" width="200"></a>
<br>
Vachan API
<br>
</h1>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't h1 , br be avoided?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When I remove h1 tag, logo image goes to left like this. Can I keep like that only or can I use div container to keep image centrally aligned.
Screenshot from 2023-11-06 13-40-04

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about not keeping it centrally aligned? And I dont know what is the point of this much care for formatting a readme file...

<img src="https://api.vachanengine.org/static/images/Logo.svg" alt="vachanapi" width="100"></a>
# VACHAN API

vachanapi

VACHAN API

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And is needed?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have asked to update readme similar to this reference

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants